Skip to content

feat: allow session config changes during a running turn - #3715

Open
testikun wants to merge 2 commits into
apache:mainfrom
testikun:codex/issue-3347-session-config
Open

feat: allow session config changes during a running turn#3715
testikun wants to merge 2 commits into
apache:mainfrom
testikun:codex/issue-3347-session-config

Conversation

@testikun

@testikun testikun commented Aug 24, 2026

Copy link
Copy Markdown

Summary

Fixes #3347.

Allow model, thinking-level, and permission-mode changes while a root Turn is running without mutating the active AgentRun.

Implementation

  • Persist a Host-owned pendingConfiguration projection with the Session header.
  • Keep the active AgentRun and its backend/execution boundary immutable.
  • Merge successive client edits against the Host pending projection, including per-model thinking-level reset/restore behavior.
  • Revalidate the pending connection, model, and thinking level at the next-Turn admission boundary; if validation fails, preserve the pending selection and report a typed error.
  • Apply the pending configuration at the fresh root-Turn admission boundary, then clear it before creating the successor AgentRun.
  • Project pending state through the Runtime Host Session catalog so reconnects, restarts, and multiple clients converge.
  • Keep Plan, Swarm, Graph, workspace, and Side Chat configuration outside this slice.

Tests

  • Runtime staging, next-admission application, and failed-validation preservation regression coverage.
  • Runtime Host model-readiness and thinking-level revalidation coverage.
  • Desktop Host-client pending merge and model/thinking reset coverage.
  • Full npm test: 1403 passed.
  • Full lint, format, typecheck, and git diff --check verification passed.

@testikun
testikun force-pushed the codex/issue-3347-session-config branch from 10a5eee to e425f73 Compare August 24, 2026 13:44

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a blocking issue that matches the pendingConfiguration path.

[P2] Pending configuration failure is not translated to a typed error

root-turn-coordinator.ts:1942 calls manager.applyPendingSessionConfiguration without catching SessionConfigurationTransitionError. The manager intentionally preserves pending and throws operation_unavailable, but prepareAdmittedTurn does not convert it via completedStart(operationUnavailable(...)). The exception bubbles out of sessionAdmission.run, so the client never receives the promised typed operation_unavailable payload and each retry re-enters the same unhandled rejection with pending permanently retained.

Fix: wrap the call and return completedStart(operationUnavailable(error.message)), consistent with other unavailable paths at line 511.

Head e425f73208. Checks on this head are currently 0 (no checks).

简体中文该 pending 失败未转 typed error,客户端无法收到明确提示。

@Astro-Han

Copy link
Copy Markdown
Contributor

Could you help clarify the product motivation for this pending configuration feature? I want to make sure the complexity is well justified:

  1. What is the concrete high-frequency workflow where changing model/thinkingLevel/permissionMode during a running turn is needed, and what is the real cost compared to "wait until the current turn finishes and then switch"?

  2. Why does the pending selection need to be durable and synchronized across restarts, reconnects and multiple clients, rather than a lighter local best-effort staging (e.g. client memory) that is dropped on reload?

  3. After the change, the controls immediately show the new values while the current Turn still runs with the old configuration, with no explicit "will take effect next turn" indicator. How is the user expected to reliably understand this, and what is the basis for that (e.g. existing pattern, user study, or prior design)?

简体中文

能否请你补充一下这个 pending 配置特性的产品动机,以便确认复杂度是否值得:

  1. 在运行中的 Turn 期间改模型/思考等级/权限模式,具体的高频工作流是什么,相比“等本轮结束再切”的真实损失在哪里?
  2. 为什么 pending 必须做成跨重启、重连、多客户端的持久状态,而不是更轻的本地 best-effort 暂存(重载即丢)?
  3. 控件切后立即显示新值,但当前 Turn 仍用旧值,且没有“下回合生效”的明确提示,用户如何可靠理解,依据是什么?

@testikun

Copy link
Copy Markdown
Author

Thanks for the review. I pushed commit 06052df with the requested fix. Root-Turn admission now catches SessionConfigurationTransitionError and returns the corresponding typed outcome (session_busy, operation_conflict, or operation_unavailable); unknown errors are still rethrown so the existing fail-stop behavior is preserved. Pending state remains persisted when application fails, and a regression test covers this path. I also fixed Runtime SessionManager summaries so pendingConfiguration remains visible to direct consumers. Product rationale: this is preparation for the next turn, not hot-swapping the active provider request. A common case is a long-running analysis, build, test, or tool-heavy turn where the user already knows the next request needs a different model, thinking level, or permission mode; waiting for the current turn adds avoidable latency, especially when a follow-up is queued. The pending selection is Host-owned Session state because the successor may be submitted by another client or admitted after reconnect/restart; renderer-local state could be lost or disagree with another client. The issue intentionally keeps the existing controls as the single configuration surface, without a separate current/next panel or persistent badge: the active AgentRun keeps its immutable snapshot, and the latest Host-authoritative value is consumed at the next root-Turn admission. This is an explicit product trade-off from the issue, not a claim based on a user study. Targeted verification on this head: SessionManager 225 passing; Root Turn Coordinator 50 passing. 中文摘要:已修复 pending 配置应用失败未返回 typed outcome 的问题;当前任务不会热切换,运行中的选择只在下一轮 root Turn admission 时生效,并由 Host 持久化以保证重连、重启和多客户端一致性。

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: the pending-configuration typed-error gap is now closed in this head.

root-turn-coordinator.ts:1942 now catches SessionConfigurationTransitionError and returns completedStart(operationUnavailable); session-manager.ts:6302 correctly projects pendingConfiguration. The new test returns a typed outcome when pending configuration cannot be applied covers the path. No remaining blocking issues.

简体中文该阻断已在该头闭合。

Astro-Han
Astro-Han previously approved these changes Aug 25, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

APPROVE — 06052df, P2 closed, test success.

@Astro-Han
Astro-Han dismissed their stale review August 25, 2026 02:26

Hold pending feature necessity review per @astrohan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow Session configuration changes while a turn is running

2 participants